Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Format with clang-format in future #10

Merged
merged 3 commits into from
Dec 30, 2023
Merged

Format with clang-format in future #10

merged 3 commits into from
Dec 30, 2023

Conversation

markuspg
Copy link
Contributor

This pull request does not introduce any functional changes to the code. I just wanted to propose to format the code automatically with clang-format in future. For this reason I added a default .clang-format file and formatted the code accordingly.

I tried to create a clang-format configuration which was better suited to the currently existing codebase than the default one. But the handling of tabs proved to be very difficult, especially for continued function parameter lists.

I guess this pull request is not for merging directly, but rather for discussion.

If there is an automatic formatting in place already and I just overlooked it, this pull request can be rejected gladly.

PS: This is not about cheaply making me the "king of git blame". Any port to a newer GTK version or another toolkit should touch most lines anyhow and flush these changes down the Git history.

@TobiX
Copy link
Owner

TobiX commented Dec 29, 2023

Good idea - I think it would be fine to migrate to some "standard" style, even if that makes the diff a lot larger (would it?). You can always add a .git-blame-ignore-revs file later (see https://docs.github.com/en/repositories/working-with-files/using-files/viewing-a-file#ignore-commits-in-the-blame-view)

@markuspg
Copy link
Contributor Author

The .clang-format I added is the default when calling clang-format --dump-config (i.e. the LLVM style). My manually adjusted one was much smaller. But the result was a bad compromise in any case, so I prefer going with the default of clang-format.

One change I conducted is creating the .clang-format file from clang-format 7 instead of clang-format 17. This maintains backwards compatibility down to Debian 10. Despite quite a few options not existing in the earlier version, the resulting formatting is identical no matter if clang-format 7 or clang-format 17 is being used with the .clang-format file.

Thank you for the hint for the .git-blame-ignore-revs file, that's a really neat feature. I added the file. Unfortunately I didn't find a way to have git blame pick it up automatically on any checkout. But at least the file exists as a hint at this feature.

@TobiX
Copy link
Owner

TobiX commented Dec 30, 2023

Unfortunately I didn't find a way to have git blame pick it up automatically on any checkout. But at least the file exists as a hint at this feature.

Yeah, it's unfortunate one can't just git config --global blame.ignoreRevsFile .git-blame-ignore-revs, because then git blame complains in repositories not containing this file. I built some automation in my own shell config, if yo need some inspiration: TobiX/dotfiles@9d1674c

@TobiX TobiX merged commit 4ee3c5d into TobiX:main Dec 30, 2023
2 checks passed
@markuspg markuspg deleted the markuspg/format branch December 30, 2023 12:39
@markuspg markuspg mentioned this pull request Dec 30, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@markuspg @TobiX